Repository navigation
fix(auth,windows): populate ID token claims and metadata - #18901
JulianPscheid wants to merge 1 commit into
Conversation
Decode the JWT payload returned by the C++ SDK and populate claims, authentication/issue/expiry times, sign-in provider and second factor when present. Keep returning the raw token if optional payload decoding fails. Use the already-linked Windows base64 decoder, exercise the parser in native tests, and verify custom claims and metadata against the Auth emulator. Enable the existing token-result integration check on Windows. Fixes firebase#11768
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements local parsing of ID token claims on Windows for firebase_auth. It introduces a custom JSON parser (IdTokenClaimsParser) and base64 decoding logic using Windows Cryptography APIs to extract claims, timestamps, and provider metadata from the ID token. It also updates integration tests in Dart and unit tests in C++ to verify the parsing behavior. Feedback on the changes includes: replacing the preprocessor macro MAXDWORD with std::numeric_limits::max() for better C++ type safety, explicitly linking the crypt32 library in CMake to ensure Windows Cryptography APIs are available, and handling potential trailing null terminators in the test helper when using CryptBinaryToStringA.
| if (c == '_') c = '/'; | ||
| } | ||
| payload.append((4 - payload.size() % 4) % 4, '='); | ||
| if (payload.size() > MAXDWORD) return std::nullopt; |
There was a problem hiding this comment.
Using std::numeric_limits<DWORD>::max() is more idiomatic in C++ and provides better type safety compared to the preprocessor macro MAXDWORD.
| if (payload.size() > MAXDWORD) return std::nullopt; | |
| if (payload.size() > (std::numeric_limits<DWORD>::max)()) return std::nullopt; |
| target_link_libraries(${TEST_RUNNER} PRIVATE | ||
| ${PLUGIN_NAME} firebase_core_plugin flutter_wrapper_plugin gtest_main) |
There was a problem hiding this comment.
Explicitly link the crypt32 library in CMake to ensure that Windows Cryptography APIs (CryptStringToBinaryA and CryptBinaryToStringA) are always available, making the target self-contained and robust against changes in dependencies.
target_link_libraries(${TEST_RUNNER} PRIVATE
${PLUGIN_NAME} firebase_core_plugin flutter_wrapper_plugin gtest_main crypt32)
| payload.resize(length); | ||
| while (!payload.empty() && payload.back() == '=') payload.pop_back(); |
There was a problem hiding this comment.
To ensure robustness across different Windows versions and environments, handle potential trailing null terminators (\0) that might be written by CryptBinaryToStringA and included in the returned length.
payload.resize(length);
while (!payload.empty() && (payload.back() == '=' || payload.back() == '\0')) {
payload.pop_back();
}
Description
On Windows,
User.getIdTokenResult()returns only the raw token from the Firebase C++ SDK. Claims, timestamps and sign-in provider are all unset, so apps can't read custom claims through the public API.This decodes the JWT payload in the Windows plugin and fills the same fields Android and Apple do: the full claims map,
authTime,issuedAtTime,expirationTime,signInProvider, andsignInSecondFactorwhen it's present. JWT timestamps are converted from seconds to milliseconds. Missing or invalid metadata stays unset, and if the payload can't be decoded the original token is still returned.The public C++ SDK headers don't expose their internal JSON parser or base64 helpers, so this uses the Windows base64 decoder that's already linked through
firebase_core, plus a small JSON-to-EncodableValue parser for the payload. No new dependency. The existing native test target now links the static plugin and tests the token-result parser, replacing its obsoletegetPlatformVersionstub. I also enabled the existing valid-token-result e2e test on Windows and added an Auth emulator test that checks custom claims, nested values, Unicode and all the token metadata it can.Testing (Flutter 3.47.9, Dart 3.13.7, Firebase C++ SDK 14.1.0):
flutter build windows --debugpasses on this branch.firebase emulators:exec --only auth --project flutterfire-e2e-tests "cd ../../../packages/firebase_auth/firebase_auth/example && flutter test integration_test/e2e_test.dart -d windows --plain-name getIdTokenResult"passes both enabled token-result tests. One existing Windows-skipped test also matches the filter.melos exec --scope=firebase_auth --scope=firebase_auth_platform_interface -c 1 -- flutter testpasses 64 and 211 tests.melos run analyze-ci --scope='firebase_auth*' --no-selectpasses for all four selected packages.flutter_plugin_tools format(with the repo's clang-format config), Dart formatting andgit diff --checkpass.To run the native tests, build the example first, configure its build directory with
-Dinclude_firebase_auth_tests=ON, build thefirebase_auth_testtarget, and runbuild/windows/x64/plugins/firebase_auth/Debug/firebase_auth_test.exe.Not covered: the full Auth e2e suite, runtime tests on other platforms, a live Auth backend, a real Windows MFA sign-in, and parser fuzzing. The tests use only the Auth emulator and the generated dummy example config.
Related Issues
Fixes #11768 (closed earlier without the Windows implementation changing).
Checklist
///).melos run analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?